From: Matthieu Gallien Date: Thu, 17 Apr 2025 12:35:02 +0000 (+0200) Subject: fix(read only folder): allow renaming new file inside read-only folders X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2^2~10^2~4 X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https://%22Program/%22http:/www.example.com/cgi/%22https:/%22Program?a=commitdiff_plain;h=aeca14f5228de35a1307646ee4c072674bad451e;p=nextcloud-desktop.git fix(read only folder): allow renaming new file inside read-only folders needed to download a new file inside a read-only folder Signed-off-by: Matthieu Gallien --- diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 660e2d382..9cae216b0 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -245,54 +245,6 @@ bool FileSystem::rename(const QString &originFileName, return success; } -bool FileSystem::uncheckedRenameReplace(const QString &originFileName, - const QString &destinationFileName, - QString *errorString) -{ -#ifndef Q_OS_WIN - bool success = false; - QFile orig(originFileName); - // We want a rename that also overwrites. QFile::rename does not overwrite. - // Qt 5.1 has QSaveFile::renameOverwrite we could use. - // ### FIXME - success = true; - bool destExists = fileExists(destinationFileName); - if (destExists && !QFile::remove(destinationFileName)) { - *errorString = orig.errorString(); - qCWarning(lcFileSystem) << "Target file could not be removed."; - success = false; - } - if (success) { - success = orig.rename(destinationFileName); - } - if (!success) { - *errorString = orig.errorString(); - qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString; - return false; - } - -#else //Q_OS_WIN - // You can not overwrite a read-only file on windows. - if (!isWritable(destinationFileName)) { - setFileReadOnly(destinationFileName, false); - } - - BOOL ok = 0; - QString orig = longWinPath(originFileName); - QString dest = longWinPath(destinationFileName); - - ok = MoveFileEx((wchar_t *)orig.utf16(), - (wchar_t *)dest.utf16(), - MOVEFILE_REPLACE_EXISTING + MOVEFILE_COPY_ALLOWED + MOVEFILE_WRITE_THROUGH); - if (!ok) { - *errorString = Utility::formatWinError(GetLastError()); - qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString; - return false; - } -#endif - return true; -} - bool FileSystem::openAndSeekFileSharedRead(QFile *file, QString *errorOrNull, qint64 seek) { QString errorDummy; diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index 8f5554000..e9547d2d0 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -132,14 +132,6 @@ namespace FileSystem { const QString &destinationFileName, QString *errorString = nullptr); - /** - * Rename the file \a originFileName to \a destinationFileName, and - * overwrite the destination if it already exists - without extra checks. - */ - bool OCSYNC_EXPORT uncheckedRenameReplace(const QString &originFileName, - const QString &destinationFileName, - QString *errorString); - /** * Removes a file. * diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index ac8a1814b..002a031de 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -633,4 +633,52 @@ FileSystem::FilePermissionsRestore::~FilePermissionsRestore() } } +bool FileSystem::uncheckedRenameReplace(const QString &originFileName, const QString &destinationFileName, QString *errorString) +{ +#ifndef Q_OS_WIN + bool success = false; + QFile orig(originFileName); + // We want a rename that also overwrites. QFile::rename does not overwrite. + // Qt 5.1 has QSaveFile::renameOverwrite we could use. + // ### FIXME + success = true; + bool destExists = fileExists(destinationFileName); + if (destExists && !QFile::remove(destinationFileName)) { + *errorString = orig.errorString(); + qCWarning(lcFileSystem) << "Target file could not be removed."; + success = false; + } + if (success) { + success = orig.rename(destinationFileName); + } + if (!success) { + *errorString = orig.errorString(); + qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString; + return false; + } +#else //Q_OS_WIN + const auto originFileInfo = QFileInfo{originFileName}; + const auto originParentFolderPath = originFileInfo.dir().absolutePath(); + FilePermissionsRestore renameEnabler{originParentFolderPath, FileSystem::FolderPermissions::ReadWrite}; + // You can not overwrite a read-only file on windows. + if (!isWritable(destinationFileName)) { + setFileReadOnly(destinationFileName, false); + } + + BOOL ok = 0; + QString orig = longWinPath(originFileName); + QString dest = longWinPath(destinationFileName); + + ok = MoveFileEx((wchar_t *)orig.utf16(), + (wchar_t *)dest.utf16(), + MOVEFILE_REPLACE_EXISTING + MOVEFILE_COPY_ALLOWED + MOVEFILE_WRITE_THROUGH); + if (!ok) { + *errorString = Utility::formatWinError(GetLastError()); + qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString; + return false; + } +#endif + return true; +} + } // namespace OCC diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 26c999994..2682f180b 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -130,6 +130,14 @@ namespace FileSystem { FileSystem::FolderPermissions permissions) noexcept; bool OWNCLOUDSYNC_EXPORT isFolderReadOnly(const std::filesystem::path &path) noexcept; + + /** + * Rename the file \a originFileName to \a destinationFileName, and + * overwrite the destination if it already exists - without extra checks. + */ + bool OWNCLOUDSYNC_EXPORT uncheckedRenameReplace(const QString &originFileName, + const QString &destinationFileName, + QString *errorString); } /** @} */ diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index 8e457e0b6..39aeb8589 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -45,6 +45,11 @@ static void assertCsyncJournalOk(SyncJournalDb &journal) #endif } +static bool isReadOnlyFolder(const std::wstring &path) +{ + return FileSystem::isFolderReadOnly(std::filesystem::path{path}); +} + SyncFileItemPtr findDiscoveryItem(const SyncFileItemVector &spy, const QString &path) { for (const auto &item : spy) { @@ -644,27 +649,21 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - auto folderStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString()); - QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(folderStatus.permissions() & std::filesystem::perms::owner_write)); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); remote.find("testFolder")->permissions = RemotePermissions::fromServerString("CKWDNVRSM"); QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - folderStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString()); - QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_write); + QVERIFY(!isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); remote.find("testFolder")->permissions = RemotePermissions::fromServerString("M"); QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - folderStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString()); - QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(folderStatus.permissions() & std::filesystem::perms::owner_write)); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); } void testChangePermissionsForFolderHierarchy() @@ -688,15 +687,9 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - auto testFolderStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString()); - QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(testFolderStatus.permissions() & std::filesystem::perms::owner_write)); - auto subFolderReadWriteStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString()); - QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_write); - auto subFolderReadOnlyStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString()); - QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_write)); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); + QVERIFY(!isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString())); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString())); remote.find("testFolder/subFolderReadOnly")->permissions = RemotePermissions::fromServerString("CKWDNVRSm"); remote.find("testFolder/subFolderReadWrite")->permissions = RemotePermissions::fromServerString("m"); @@ -708,12 +701,9 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - subFolderReadWriteStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString()); - QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_write)); - subFolderReadOnlyStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString()); - QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_write); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString())); + QVERIFY(!isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString())); remote.rename("testFolder/subFolderReadOnly", "testFolder/subFolderReadWriteNew"); remote.rename("testFolder/subFolderReadWrite", "testFolder/subFolderReadOnlyNew"); @@ -722,9 +712,7 @@ private slots: QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - testFolderStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString()); - QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read); - QVERIFY(!static_cast(testFolderStatus.permissions() & std::filesystem::perms::owner_write)); + QVERIFY(isReadOnlyFolder(static_cast(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString())); } void testDeleteChildItemsInReadOnlyFolder()